fix: improve Ledger error handling for disconnect, retry, and pagination cp-7.77.0#28515
Conversation
…ion (#28272) - Force BLE cleanup in LedgerBluetoothAdapter#closeTransport even when transport is null but deviceId exists, preventing stale OS-level BLE connections from blocking reconnection after device power-cycle. - Expand transient BLE error detection with message-based fallback so reconnection errors with generic Error names are retried instead of immediately surfaced as "Something went wrong". - Close transport in #handleWrongApp when openEthereumApp/closeRunningApp fails (user cancelled on device), preventing stale transport on retry. - Add ensureDeviceReady check before pagination in LedgerSelectAccount (nextPage/prevPage), surfacing the HW wallet bottom sheet for reconnection instead of showing cryptic "Unspecified error" inline. - Add disconnect error detection in getLedgerAccountsByOperation before the generic catch-all to provide clearer error messages.
- Added `.cursor/hooks/state/` to `.gitignore` to prevent tracking of local state files generated by the Cursor tool.
|
CLA Signature Action: All authors have signed the CLA. You may need to manually re-run the blocking PR check if it doesn't pass in a few minutes. |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
github-actions
Bot
added
the
risk-medium
…ssifying non-transient errors The message-based fallback in #isTransientBleError matched any error containing 'bluetooth' (case-insensitive), which misclassified non-transient Bluetooth errors (permission failures, 'Bluetooth not supported', 'Bluetooth is off') as transient. This triggered up to two unnecessary retries with RETRY_DELAY_MS delays before the real error surfaced. Replace the broad 'bluetooth' pattern with specific connectivity-related patterns: 'bluetooth connection' and 'bluetooth transfer'. The existing 'disconnected', 'connection lost', and 'gatt' patterns already cover most BLE connectivity issues. Also adds negative tests ensuring non-transient bluetooth errors like permission denials and disabled-state errors are not retried. Co-authored-by: Xiaoming Wang <dawnseeker8@users.noreply.github.com>
github-actions
Bot
added
risk-medium
Replace the generic 'bluetooth' substring match with more specific 'bluetooth connection' and 'ble error' patterns to avoid misclassifying non-transient errors (e.g. 'Bluetooth not supported', permission failures) as transient, which would trigger unnecessary retries. Add test to verify non-transient Bluetooth errors are not retried. Co-authored-by: Xiaoming Wang <dawnseeker8@users.noreply.github.com>
github-actions
Bot
added
risk-medium
|
Thanks for the feedback @owencraston. Tested on Android and iOS, here is feedback for all the issues: issue1 - Send trx while the ledger is disconnected: The “ledger disconnected” error doesn’t appear, but the issue is reproducible only if the user taps “continue” before the Connection Lost screen disappears by itself after turning on the ledger. After that, if the user continues with the scenario, the trx will be success. The video is attached in the slack thread https://consensys.slack.com/archives/C05C8481HSB/p1775639674912209 , since the file size is too big to be attached here. issue2 - Perform dapp signing while the ledger is disconnected: The issue is reproducible. Video attached. 101.movISSUE2: [Bug]: [Ledger] "Something went wrong" error is shown when the user tries to perform dapp signing while the ledger ETH “Blind singing” is disabled Fixed. ISSUE3: ISSUE3: "Bluetooth required" modal is presented even though the bluetooth is already enabled on both the Mobile and the Ledger device: The issue is reproducible. Video attached. 102.MP4ISSUE4: "Ledger disconnected" error is shown on Send/Swap/Bridge/Dapp sign if the user denies to open the ETH app in the first try Fixed. ISSUE5: "Unspecified error" message is shown after trying to use the pagination in the Select an Account screen if the ETH app is not open Fixed. |
github-actions
Bot
added
risk-low
Avoid false Bluetooth-required gating when entering scan mode without a device ID, and parse plain-object Ledger errors by name/status so disconnects map to actionable hardware wallet errors instead of generic unknown failures. Made-with: Cursor
github-actions
Bot
added
risk-medium
HI, @nikolastoimenovski-consensys i am unable to replicate the issue 3 in above list. can you clarify whether you have given metamask access bluetooth permission? |
- Updated the mock implementation for `getFirstPage` and `getNextPage` in `Ledger.test.ts` to use `jest.mocked` for improved clarity and consistency in error handling tests related to disconnected devices.
dawnseeker8
changed the title
NicolasMassart
left a comment
NicolasMassart
left a comment
There was a problem hiding this comment.
Inline review finding on the new hook test.
NicolasMassart
left a comment
NicolasMassart
left a comment
There was a problem hiding this comment.
Inline review finding on pagination disconnect classification.
Co-authored-by: Nico MASSART <NicolasMassart@users.noreply.github.com>
Co-authored-by: Nico MASSART <NicolasMassart@users.noreply.github.com>
github-project-automation
Bot
moved this from Needs dev review
to Review finalised - Ready to be merged
in PR review queue
- Adjusted formatting of error message handling for improved readability and consistency in the `getLedgerAccountsByOperation` function within `Ledger.ts`.
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, have a team admin enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 2f9e7f2. Configure here.
🔍 Smart E2E Test Selection
click to see 🤖 AI reasoning detailsE2E Test Selection:
Tag Selection Rationale:
Confidence note: Confidence is 78 because there are no dedicated Ledger E2E tests in the suite, so SmokeAccounts is the best available proxy for hardware wallet account management coverage. Performance Test Selection: |
|
metamaskbotv2
Bot
added
the
release-7.78.0
5 participants
Description
Fixes multiple issues with Ledger hardware wallet error handling that caused confusing error dialogs when the device was disconnected, the Ethereum app was closed, or blind signing was disabled:
#closeTransportnow forces BLE disconnection even when the transport object is already null but a device ID remains, preventing the OS from keeping a stale connection that blocks reconnection.#isTransientBleErrornow falls back to message-based matching (e.g. "disconnected", "bluetooth", "gatt") for BLE errors that use genericErrornames after a device power-cycle.#handleWrongAppnow closes the transport whenopenEthereumAppOnLedgerorcloseRunningAppOnLedgerfails, preventing stale connections.nextPage/prevPageinLedgerSelectAccountnow callensureDeviceReadybefore fetching accounts, and only dismiss the blocking modal if it was actually shown (via amodalShownflag).getLedgerAccountsByOperationnow detects disconnect errors and throws a user-friendly "device got disconnected" message instead of a generic error..cursor/hooks/state/to.gitignoreto prevent Cursor IDE state files from being committed.Changelog
CHANGELOG entry: Fixed Ledger hardware wallet error handling to properly recover from disconnects, app switching failures, and pagination errors
Related issues
Fixes: #28272
Manual testing steps
Screenshots/Recordings
Before
N/A
After
N/A
Pre-merge author checklist
Pre-merge reviewer checklist
Note
Medium Risk
Changes touch Ledger BLE transport cleanup/retry logic and account-pagination readiness gating, which can affect hardware-wallet connection stability and user flows. Risk is mitigated by expanded unit test coverage across adapter, error parsing, and UI pagination cases.
Overview
Improves Ledger Bluetooth robustness by forcing BLE cleanup even when the transport object is already cleared, and by expanding transient disconnect detection to include message-based matches; it also closes the transport when app-switch commands fail to avoid stale connections.
Updates
LedgerSelectAccountpagination (nextPage/prevPage) to callensureDeviceReadybefore fetching pages and to only dismiss the blocking modal if it was shown, preventing unnecessary modal flicker and avoiding pagination attempts when the device isn’t ready.Enhances Ledger error classification by treating disconnect-like failures in
getLedgerAccountsByOperationas a user-facing “disconnected” error, and broadensparseErrorByTypeto handle non-Errorobjects with Ledger-shapedname/statusCode; adds/updates unit tests across these scenarios.Reviewed by Cursor Bugbot for commit 5e139f5. Bugbot is set up for automated code reviews on this repo. Configure here.